Conversation
… overflow - Fix argument register clobbering in function calls by evaluating all args to stack temporaries before loading registers - Fix LEFT$/RIGHT$/MID$ builtins to use callee-saved registers (r12-r14) - Support >6 parameters in SUB/FUNCTION via stack-based overflow args - Add GOSUB stack overflow protection with runtime error handler - Increase GOSUB stack from 1K to 64K entries (512KB) - Add expression nesting depth warning at 256 levels - Use consistent sub rsp/add rsp pattern for string concatenation temps - Add integration tests for all fixes Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Combine related test cases into single BASIC programs with multi-line output validation to reduce compile-link cycles. Also fix clippy warnings about approximate PI values in lexer tests. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR consolidates test suites and implements bug fixes for GOSUB stack overflow handling, procedure parameter overflow (>6 params), and string function argument evaluation. The changes reduce test file verbosity by combining related tests while maintaining comprehensive coverage.
Changes:
- Consolidated 11 test modules by combining multiple small tests into larger, more efficient test functions
- Added GOSUB stack overflow detection with error handling and increased stack size from 8KB to 512KB
- Fixed parameter passing for procedures with more than 6 arguments (register overflow to stack)
- Fixed string function (LEFT$, RIGHT$, MID$) argument evaluation using callee-saved registers to prevent register clobbering
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/variables/mod.rs | Consolidated 7 variable tests into 2 comprehensive tests |
| tests/types/mod.rs | Consolidated 53 type promotion/conversion tests into 6 focused tests |
| tests/strings/mod.rs | Consolidated 6 string function tests into 3 tests with nested calls |
| tests/procedures/mod.rs | Consolidated 3 procedure tests into 4 tests including stress tests |
| tests/print/mod.rs | Consolidated 3 print tests into 1 combined test |
| tests/math/mod.rs | Consolidated 59 math function tests into 10 type-focused tests |
| tests/input/mod.rs | Consolidated 2 input tests into 1 test |
| tests/data/mod.rs | Consolidated 2 DATA/READ tests into 1 test |
| tests/control/mod.rs | Consolidated 17 control flow tests into 7 tests plus GOSUB stress test |
| tests/arrays/mod.rs | Consolidated 3 array tests into 1 test covering 1D and 2D arrays |
| tests/arithmetic/mod.rs | Consolidated 41 arithmetic tests into 8 operator-focused tests |
| src/runtime/win64-native/print.s | Added _rt_gosub_overflow error handler for Win64 |
| src/runtime/win64-native/data_defs.s | Added GOSUB overflow error message |
| src/runtime/sysv/print.s | Added _rt_gosub_overflow error handler for SysV |
| src/runtime/sysv/data_defs.s | Added GOSUB overflow error message |
| src/lexer.rs | Updated test float literals from 3.14159 to avoid precision issues |
| src/codegen.rs | Fixed GOSUB overflow detection, parameter overflow handling, string function register preservation, and expression depth tracking |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.